Skip to content

Conversation

@nhedger
Copy link

@nhedger nhedger commented Jun 20, 2022

Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nhedger Changes LGTM, but I'm not sure how much sense it makes to update the legacy HttpClient component. This component has been deprecated as per #152 circa 2 years ago.

@nhedger
Copy link
Author

nhedger commented Jun 20, 2022

Made sense to me because the first code block gives instructions to install the new reactphp/http component but I guess it's not absolutely necessary.

@WyriHaximus WyriHaximus requested a review from clue June 24, 2022 09:27
Copy link
Member

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can still remove it here to be honest. It won't hurt anyone :).

@clue
Copy link
Member

clue commented Jul 5, 2022

I don't have any strong opinions on this one and I'm still not sure what to do about this PR, but I'm currently leaning towards not maintaining this component anymore as per #152/#153.

GitHub prominently features the date the project and individual files have been last updated, so I feel merging this PR might come over as this project being "resurrected". I'd rather convey the fact that this project is in fact "dead" prominently.

It's my understanding that if we accept this PR, there's reason to believe we should do more housekeeping for this project, like updating the failing test suite and more. Even if we don't do this ourselves, there's a chance somebody else might look into this and reasonably ask for a PR review. Personally, I'd rather spend time on working the current HTTP component instead.

@nhedger
Copy link
Author

nhedger commented Jul 5, 2022

I agree. Wouldn't it make sense to archive the repository in that case ?

@nhedger nhedger closed this by deleting the head repository Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants